Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable GetGCMemoryInfo on arm #73477

Merged
merged 1 commit into from
Aug 8, 2022
Merged

Disable GetGCMemoryInfo on arm #73477

merged 1 commit into from
Aug 8, 2022

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Aug 5, 2022

Disables test blocking CI #73247

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 5, 2022
@hoyosjs hoyosjs marked this pull request as ready for review August 5, 2022 17:30
@ghost ghost assigned hoyosjs Aug 5, 2022
@ghost
Copy link

ghost commented Aug 5, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Disables test blocking CI #73247

Author: hoyosjs
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@hoyosjs hoyosjs added area-GC-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Aug 5, 2022
@ghost
Copy link

ghost commented Aug 5, 2022

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Disables test blocking CI #73247

Author: hoyosjs
Assignees: hoyosjs
Labels:

area-GC-coreclr
Milestone: -

@hoyosjs hoyosjs mentioned this pull request Aug 5, 2022
Closed
@hoyosjs hoyosjs requested review from a team August 5, 2022 17:33
@cshung
Copy link
Member

cshung commented Aug 5, 2022

@hoyosjs, Can we test this fix instead? #73247

@hoyosjs
Copy link
Member Author

hoyosjs commented Aug 5, 2022

Seems like a reasonable workaround - is it OK to not have accounting on win-x86?

@hoyosjs
Copy link
Member Author

hoyosjs commented Aug 5, 2022 •
edited
Loading

I'll leave this open and merge later if the other PR is not approved by the time this CI is done. We can easily add the revert to your PR, but this issue is hitting at a high rate.

@cshung
Copy link
Member

cshung commented Aug 5, 2022

Seems like a reasonable workaround - is it OK to not have accounting on win-x86?

Yes - heap_hard_limit is supported on 64 bit platforms only

@hoyosjs
Copy link
Member Author

hoyosjs commented Aug 5, 2022

Ah, that'd be a proper fix altogether then.

@mangod9
Copy link
Member

mangod9 commented Aug 5, 2022

what in that change was causing the ArgumentOutOfRange exception though?

@cshung
Copy link
Member

cshung commented Aug 5, 2022

what in that change was causing the ArgumentOutOfRange exception though?

The test case itself did it - and that was not a problem at all.

@hoyosjs hoyosjs closed this Aug 5, 2022
@hoyosjs hoyosjs reopened this Aug 5, 2022
@hoyosjs hoyosjs merged commit 9865cc7 into main Aug 8, 2022
@hoyosjs hoyosjs deleted the juhoyosa/disable-gc-getinfo branch August 8, 2022 17:02
@am11 am11 mentioned this pull request Aug 8, 2022
Merged
noahfalk added a commit to noahfalk/runtime that referenced this pull request Aug 8, 2022
@noahfalk
Revert "Disable GetGCMemoryInfo on arm (dotnet#73477)"
8694661 
This reverts commit 9865cc7.
We believe the problematic change has been reverted so the test can be re-enabled.
hoyosjs pushed a commit that referenced this pull request Aug 9, 2022
@noahfalk
Revert "Disable GetGCMemoryInfo on arm (#73477)" (#73595)
d0aa175 
This reverts commit 9865cc7.
We believe the problematic change has been reverted so the test can be re-enabled.
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mangod9 mangod9 mangod9 approved these changes

Assignees

@hoyosjs hoyosjs

Labels
area-GC-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hoyosjs @cshung @mangod9